feat(calendar): rename calendar from sidebar, pushing to remote#110
Merged
feat(calendar): rename calendar from sidebar, pushing to remote#110
Conversation
Code in db/folders.rs, jmap_sync.rs, and friends INSERTs into folders.parent_id but the column was never added to the CREATE TABLE or the migration list. Existing DBs had it via an orphaned ad-hoc migration from an earlier build; fresh installs blew up with "table folders has no column named parent_id" on first JMAP folder upsert. Add the ALTER migration so fresh installs match existing ones.
Add a Rename… item to the calendar right-click context menu. The new
update_calendar path loads the calendar + account, and when the name
changes, pushes the new name to the remote server before writing the
local DB:
- CalDAV: PROPPATCH {DAV:}displayname on the calendar collection URL
- JMAP: Calendar/set with an update entry
- Graph: PATCH /me/calendars/{id}
- Gmail: Google Calendar REST PATCH /calendars/{id}, falling back to
CalDAV PROPPATCH if OAuth isn't configured
Local-only calendars (no remote_id) still rename, just without a
remote round-trip. On remote failure the DB stays unchanged so the
user can retry.
Also replace the fragile @click-on-sidebar-root close handler with a
document-level click-outside listener that ignores right-button click
events (WebKitGTK synthesises a click on right-mouse-release, which
used to slam the menu shut the moment it opened).
Closes #44.
CI uses a newer rustfmt that prefers method-chain formatting on the column probe. Match it.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds calendar renaming from the sidebar (with remote propagation) and fixes a missing DB migration, while also adjusting context-menu dismissal behavior to avoid WebKitGTK right-click-release quirks.
Changes:
- Add “Rename…” to the calendar sidebar context menu with a modal UI and inline error display.
- Update backend
update_calendarto push renames to the appropriate remote (JMAP/Graph/Google/CalDAV) before mutating the local DB. - Add a migration to ensure
folders.parent_idexists for fresh installs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/calendar/CalendarSidebar.vue | Adds rename UI/modal and replaces sidebar click handler with a document-level click-outside listener. |
| src-tauri/src/commands/calendar.rs | Pushes calendar renames to remote providers before local DB update; adds protocol dispatch helper. |
| src-tauri/src/mail/jmap.rs | Introduces JMAP Calendar/set rename helper. |
| src-tauri/src/mail/graph.rs | Adds Graph calendar rename helper via PATCH. |
| src-tauri/src/mail/caldav.rs | Adds CalDAV PROPPATCH rename helper and XML escaping utility. |
| src-tauri/src/db/schema.rs | Adds migration for folders.parent_id. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+306
to
+316
| // Add parent_id column to folders. Existing DBs that were populated by | ||
| // older JMAP sync builds already had it; fresh installs didn't because | ||
| // the CREATE TABLE in initialize() was never updated to match. Without | ||
| // this column the first JMAP folder upsert fails with "no column named | ||
| // parent_id". | ||
| let has_folder_parent_id: bool = conn | ||
| .prepare("SELECT parent_id FROM folders LIMIT 0") | ||
| .is_ok(); | ||
| if !has_folder_parent_id { | ||
| log::info!("Migration: adding parent_id column to folders table"); | ||
| conn.execute_batch("ALTER TABLE folders ADD COLUMN parent_id TEXT;")?; |
There was a problem hiding this comment.
This migration adds folders.parent_id, but initialize()'s CREATE TABLE folders still doesn't include parent_id. As a result, every fresh DB will always need the migration to reach the expected schema, and the schema definition stays out of sync with the runtime expectations. Update the CREATE TABLE IF NOT EXISTS folders statement to include parent_id TEXT so new installs start with the correct schema.
1. JMAP Calendar/set used the literal key "calendar_id" instead of the
runtime id. serde_json::json!({ identifier: ... }) stringifies the
identifier name, not its value. Build the update map with
serde_json::Map + insert(id.to_string(), ...), matching the pattern
already used by update_calendar_event in the same file. Any apparent
successful rename before this fix was either a server-side leniency
or happened against a local-only calendar.
2. Add parent_id to the folders CREATE TABLE so fresh installs match
the expected schema; the migration now becomes a no-op on new DBs
while still repairing installs seeded by older builds.
3. Rename the global .modal-overlay class the rename modal was using to
.cal-rename-overlay so it cannot clash with the several scoped
.modal-overlay rules already in other components.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
update_calendarnow loads the calendar + account and, when the name changes, pushes the new name to the remote server before touching local DB:{DAV:}displaynameon the calendar collection URL.Calendar/setwith an update entry./me/calendars/{id}./calendars/{id}, falling back to CalDAV PROPPATCH if OAuth isn't configured.remote_id) still rename locally. On remote failure the DB is left unchanged so the user can retry.@clickclose-on-sidebar-click handler with a document-level click-outside listener that guards onbutton === 0. WebKitGTK synthesises a click event on right-mouse-release; the old handler interpreted that as an outside-click and slammed the menu shut on release.ALTER TABLE folders ADD COLUMN parent_id TEXTmigration. Existing DBs had this column from an orphaned earlier migration, but fresh installs would crash with "table folders has no column named parent_id" on first JMAP folder upsert. Preserved as a separate commit so the review diff is focused.Closes #44.
Test plan
cargo build --all-targets,cargo clippy --all-targets -- -D warnings,cargo fmt --check: cleanpnpm test(229 passed),pnpm exec vue-tsc --noEmit: cleanJMAP rename calendar: id=… -> …/JMAP renamed calendar ….